From 1f56f5c2e97afdd6ab75a0826f13ea8608e8246c Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 11 Feb 2019 04:57:46 +0100 Subject: [PATCH] selectionmodel: Change prototype of query_range() 1. Do not make position an inout variable The function is meant to return a range for a given position, not modify a position. So it makes no conceptual sense to use an inout variable. 2. Pass the selected state as an out variable Using a boolean return value - in particular in an interface full of boolean return values - makes the return value intuitively feel like a success/failure return. Using an out variable clarifies the usage. 3. Allow passing every position value Define what happens when position >= list.n_items 4. Clarify the docs about how this function should behave In particular, mention the case from point (3) 5. Add more tests Again, (3) needs testing. --- gtk/gtkselectionmodel.c | 54 ++++++++++++++++++++++++--------- gtk/gtkselectionmodel.h | 16 ++++++---- gtk/gtksingleselection.c | 47 +++++++++++++++++----------- testsuite/gtk/singleselection.c | 9 ++++-- 4 files changed, 86 insertions(+), 40 deletions(-) diff --git a/gtk/gtkselectionmodel.c b/gtk/gtkselectionmodel.c index 137928c687..cc0823d1c6 100644 --- a/gtk/gtkselectionmodel.c +++ b/gtk/gtkselectionmodel.c @@ -120,13 +120,25 @@ gtk_selection_model_default_unselect_all (GtkSelectionModel *model) return gtk_selection_model_unselect_range (model, 0, g_list_model_get_n_items (G_LIST_MODEL (model)));; } -static gboolean +static void gtk_selection_model_default_query_range (GtkSelectionModel *model, - guint *position, - guint *n_items) + guint position, + guint *start_range, + guint *n_items, + gboolean *selected) { - *n_items = 1; - return gtk_selection_model_is_selected (model, *position); + *start_range = position; + + if (position >= g_list_model_get_n_items (G_LIST_MODEL (model))) + { + *n_items = 0; + *selected = FALSE; + } + else + { + *n_items = 1; + *selected = gtk_selection_model_is_selected (model, position); + } } static void @@ -266,27 +278,41 @@ gtk_selection_model_unselect_all (GtkSelectionModel *model) /** * gtk_selection_model_query_range: * @model: a #GtkSelectionModel - * @position: (inout): specifies the position on input, and the first element of the range on output + * @position: the position inside the range + * @start_range: (out): returns the position of the first element of the range * @n_items: (out): returns the size of the range + * @selected: (out): returns whether items in @range are selected * * This function allows to query the selection status of multiple elements at once. * It is passed a position and returns a range of elements of uniform selection status. - * The returned range is guaranteed to include the passed-in position. - * The selection status is returned from this function. * - * Returns: %TRUE if the elements in the returned range are selected, %FALSE otherwise + * If @position is greater than the number of items in @model, @n_items is set to 0. + * Otherwise the returned range is guaranteed to include the passed-in position, so + * @n_items will be >= 1. + * + * Positions directly adjacent to the returned range may have the same selection + * status as the returned range. + * + * This is an optimization function to make iterating over a model faster when few + * items are selected. However, it is valid behavior for implementations to use a + * naive implementation that only ever returns a single element. */ -gboolean +void gtk_selection_model_query_range (GtkSelectionModel *model, - guint *position, - guint *n_items) + guint position, + guint *start_range, + guint *n_items, + gboolean *selected) { GtkSelectionModelInterface *iface; - g_return_val_if_fail (GTK_IS_SELECTION_MODEL (model), FALSE); + g_return_if_fail (GTK_IS_SELECTION_MODEL (model)); + g_return_if_fail (start_range != NULL); + g_return_if_fail (n_items != NULL); + g_return_if_fail (selected != NULL); iface = GTK_SELECTION_MODEL_GET_IFACE (model); - return iface->query_range (model, position, n_items); + return iface->query_range (model, position, start_range, n_items, selected); } void diff --git a/gtk/gtkselectionmodel.h b/gtk/gtkselectionmodel.h index 65c424716d..9e8de6a66b 100644 --- a/gtk/gtkselectionmodel.h +++ b/gtk/gtkselectionmodel.h @@ -79,9 +79,11 @@ struct _GtkSelectionModelInterface guint n_items); gboolean (* select_all) (GtkSelectionModel *model); gboolean (* unselect_all) (GtkSelectionModel *model); - gboolean (* query_range) (GtkSelectionModel *model, - guint *position, - guint *n_items); + void (* query_range) (GtkSelectionModel *model, + guint position, + guint *start_range, + guint *n_items, + gboolean *selected); }; GDK_AVAILABLE_IN_ALL @@ -110,9 +112,11 @@ GDK_AVAILABLE_IN_ALL gboolean gtk_selection_model_unselect_all (GtkSelectionModel *model); GDK_AVAILABLE_IN_ALL -gboolean gtk_selection_model_query_range (GtkSelectionModel *model, - guint *position, - guint *n_items); +void gtk_selection_model_query_range (GtkSelectionModel *model, + guint position, + guint *start_range, + guint *n_items, + gboolean *selected); /* for implementations only */ GDK_AVAILABLE_IN_ALL diff --git a/gtk/gtksingleselection.c b/gtk/gtksingleselection.c index 17ae761c73..642c79847a 100644 --- a/gtk/gtksingleselection.c +++ b/gtk/gtksingleselection.c @@ -133,36 +133,47 @@ gtk_single_selection_unselect_item (GtkSelectionModel *model, return TRUE; } -static gboolean +static void gtk_single_selection_query_range (GtkSelectionModel *model, - guint *position, - guint *n_items) + guint position, + guint *start_range, + guint *n_range, + gboolean *selected) { GtkSingleSelection *self = GTK_SINGLE_SELECTION (model); + guint n_items; - if (self->selected == GTK_INVALID_LIST_POSITION) + n_items = g_list_model_get_n_items (self->model); + + if (position >= n_items) + { + *start_range = position; + *n_range = 0; + *selected = FALSE; + } + else if (self->selected == GTK_INVALID_LIST_POSITION) { - *position = 0; - *n_items = g_list_model_get_n_items (self->model); - return FALSE; + *start_range = 0; + *n_range = n_items; + *selected = FALSE; } - else if (*position < self->selected) + else if (position < self->selected) { - *position = 0; - *n_items = self->selected; - return FALSE; + *start_range = 0; + *n_range = self->selected; + *selected = FALSE; } - else if (*position > self->selected) + else if (position > self->selected) { - *position = self->selected + 1; - *n_items = g_list_model_get_n_items (self->model) - *position; - return FALSE; + *start_range = self->selected + 1; + *n_range = n_items - *start_range; + *selected = FALSE; } else { - *position = self->selected; - *n_items = 1; - return TRUE; + *start_range = self->selected; + *n_range = 1; + *selected = TRUE; } } diff --git a/testsuite/gtk/singleselection.c b/testsuite/gtk/singleselection.c index efe4a91ece..3ce754bbe9 100644 --- a/testsuite/gtk/singleselection.c +++ b/testsuite/gtk/singleselection.c @@ -495,14 +495,19 @@ check_query_range (GtkSelectionModel *selection) /* check that range always contains position, and has uniform selection */ for (i = 0; i < g_list_model_get_n_items (G_LIST_MODEL (selection)); i++) { - position = i; - selected = gtk_selection_model_query_range (selection, &position, &n_items); + gtk_selection_model_query_range (selection, i, &position, &n_items, &selected); g_assert_cmpint (position, <=, i); g_assert_cmpint (i, <, position + n_items); for (j = position; j < position + n_items; j++) g_assert_true (selected == gtk_selection_model_is_selected (selection, j)); } + /* check that out-of-range returns the correct invalid values */ + i = MIN (i, g_random_int ()); + gtk_selection_model_query_range (selection, i, &position, &n_items, &selected); + g_assert_cmpint (position, ==, i); + g_assert_cmpint (n_items, ==, 0); + g_assert_true (!selected); } static void -- 2.30.2